fix: tools-call-simple-text now fails when tool returns isError: true#165
fix: tools-call-simple-text now fails when tool returns isError: true#165lux999 wants to merge 3 commits intomodelcontextprotocol:mainfrom
Conversation
commit: |
Head branch was pushed to by a user without write access
The reference everything-server's zod schema rejects `undefined` arguments
and requires at least `{}`, so omitting `arguments` caused the server to
return `isError: true` with a validation error. This went unnoticed under
the previous lenient check but is correctly flagged by the new `isError`
assertion. Matches the convention used by every other tool-call scenario
in this file.
lux999
force-pushed
the
fix/tools-call-simple-text-iserror-check
branch
from
04febed to
3af5ac4
Compare
|
Hi @pcarleton, thanks for the approval! Quick heads-up on the new commit: after merging main, CI caught one test failure. This scenario originally omitted I squashed a small workaround into the fix commit: pass Thanks again! |
2 participants
Added an
isErrorcheck toToolsCallSimpleTextScenario, so the testtools-call-simple-textcorrectly fails when the tool returns an error result instead of a successful text response.Motivation and Context
The test
tools-call-simple-textwas passing for servers that don't implementtest_simple_textat all.When an unknown tool is called, MCP frameworks return
{ isError: true, content: [{ type: "text", text: "Tool not found: ..." }] }. The existing assertions only checked for a non-empty text content item, which the "Tool not found" error message satisfies. The missing assertion was thatresult.isErrormust not be true.How Has This Been Tested?
Verified against the existing everything-server reference implementation, which correctly implements
test_simple_textand continues to pass. Confirmed that servers withouttest_simple_textnow correctly fail the test.Breaking Changes
Servers that were previously passing
tools-call-simple-textwithout implementingtest_simple_textwill now correctly fail.Types of changes
Checklist
Additional context